Resolve siteId dynamically via x-site-id header for multisite#3700
Conversation
…logout When HttpOnly session cookies are enabled, the shopper's access token and refresh token are stored in HttpOnly cookies and are not accessible to client-side JavaScript. The SLAS /oauth2/logout endpoint requires both a Bearer token in the Authorization header and a refresh_token query parameter. This change injects both from HttpOnly cookies in the SLAS private client proxy. Also moves SLAS-specific Bearer token logic out of configure-proxy.js (regular /mobify/proxy path) into the SLAS private client proxy where it belongs, since SLAS calls don't go through the regular proxy when useSLASPrivateClient is enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With multisite enabled, siteId is request-dependent but was previously read once at startup from static config, breaking HttpOnly cookie lookups for non-default sites. This adds an x-site-id header from the per-request resolved site and reads it in the proxy layers with static config fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
resolveSiteFromUrl already falls back to the default site, so the x-site-id header always carries a valid value. Remove the static config fallback and the now-unused siteId parameter from the proxy chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vcua-mobify
left a comment
vcua-mobify
left a comment
There was a problem hiding this comment.
Thanks for following up on this. Using a header to pass in the site id seems like a good approach to me.
There was a problem hiding this comment.
Since this is an _app-config change, we'll want to update the _app-config templates in the generator.
We'll probably also need to let existing projects that are consuming this change know that they need to add this header
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eck, warn on missing x-site-id Only invoke applyScapiAuthHeaders when HttpOnly session cookies are enabled. Remove the unnecessary SLAS auth endpoint guard since SLAS requests use a separate proxy. Log a warning when x-site-id header is missing on SCAPI requests to aid debugging if the template is misconfigured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tract injectLogoutTokens The option only matches the logout endpoint and injects both Bearer token and refresh token, so the old name was misleading. Extract the inline logout injection logic into a standalone injectLogoutTokens function for clarity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| * sets the Authorization header, and appends refresh_token to the query string. | ||
| * @private | ||
| */ | ||
| export const injectLogoutTokens = (proxyRequest, incomingRequest) => { |
There was a problem hiding this comment.
minor refactor - moved to a separate function
There was a problem hiding this comment.
Should this function be moved into process-token-response?
There was a problem hiding this comment.
it is not really related to processing the response. Other option is to rename process-token-response to a more generic one - httponly-cookie-utils and then move this function. I will take another look and see if it make sense to rename
There was a problem hiding this comment.
I will leave it as is for now and consider rename/refactor as we make more changes
vcua-mobify
left a comment
vcua-mobify
left a comment
There was a problem hiding this comment.
Thanks for making the changes @unandyala !
Just a few more comments then I think this is ready
packages/pwa-kit-runtime/src/ssr/server/process-token-response.test.js
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
| } | ||
| injectLogoutTokens(proxyRequest, incomingRequest) |
There was a problem hiding this comment.
Nit: Can we make this func a bit clearer: injectTokenToLogoutEndpoint? and it would be helpful to explain why
// SLAS log out endpoint requires token and refresh token in the body, the other SLAS does not need to pass token in the body
injectTokenToLogoutEndpoint(...)
unandyala
merged commit c45512d
into
feature/httponly-session-cookies
5 participants
Summary
x-site-idheader toCommerceApiProviderheaders from the per-request resolvedlocals.site?.id, so every SDK request carries the correct siteIdbuild-remote-server.js), token response processing (process-token-response.js), and regular proxy auth (configure-proxy.js) to preferx-site-idheader over static config siteId, with static config as fallback for backwards compatibilitycc-at_{siteId},cc-nx_{siteId}) that were always using the default site's ID, breaking token injection for non-default sites in multisite setupshttponly-multisite.mov
Test plan
process-token-response.test.js— 22/22 passed (2 new: header override + fallback)configure-proxy.basic.test.js— 12/12 passed (2 new: header override + fallback)build-remote-server.test.js— 54/54 passed (1 new: header precedence over static config for logout)🤖 Generated with Claude Code